fix #4251: wrap BackgroundTasks in their own span#4368
Open
davidgss04 wants to merge 1 commit intoopen-telemetry:mainfrom
Open
fix #4251: wrap BackgroundTasks in their own span#4368davidgss04 wants to merge 1 commit intoopen-telemetry:mainfrom
davidgss04 wants to merge 1 commit intoopen-telemetry:mainfrom
Conversation
|
|
Contributor
|
Thanks for this! The fix to Please could you also commit fixes from:
|
Only the request/ASGI middleware was traced; background tasks were not. Background tasks ran after the response with no span, causing child spans to attach to a closed parent span. This change patches BackgroundTask.__call__ to wrap execution in a new span. Each background task now creates a child span of the request, ensuring a correct trace hierarchy. Add three regression tests for background task instrumentation.
davidgss04
force-pushed
the
fix-bug-4251
branch
from
d65500d to
ab9b962
Compare
Author
Good evening, I have applied the requested |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #4251
FastAPI background tasks were not properly instrumented. While request spans were created correctly, background tasks executed after the response without their own span. As a result, any spans created inside a background task were incorrectly attached directly to the request span, which had already finished, leading to broken trace hierarchies and inaccurate timing information.
The fix patches
BackgroundTask.__call__to wrap its execution in a dedicated span. This ensures that each background task runs within its own span, correctly parented to the request span, and that any spans created inside the task are properly nested.The patch is applied only once using a guard (
hasattr) to avoid double instrumentation, and the original method is restored duringuninstrument_appto prevent side effects.Type of change
Testing
Added three regression tests:
test_background_task_span_parents_inner_spans— verifies that background tasks create a wrapper span and that spans created inside the task are correctly parentedtest_uninstrument_app_restores_background_task_call— ensures the originalBackgroundTask.__call__is restored after uninstrumentationtest_background_task_span_not_duplicated_on_double_instrument_app— verifies that repeated instrumentation does not create duplicate spans or apply the patch multiple timesDoes This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.